Assessment Flow (Middle Section - minimal intro and outro)#271
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
3e71e12 to
0aa3861
Compare
| className="flex w-full items-center gap-3 text-left" | ||
| > | ||
| {isLoading && ( | ||
| <Loader2 className="text-sarge-gray-500 size-4.5 flex-shrink-0 animate-spin" /> |
There was a problem hiding this comment.
would it be better to use ternary logic instead of the && operator here @cherman23 @LOTaher
| @@ -0,0 +1,17 @@ | |||
| import { HighlightGuard } from '@/lib/components/core/HighlightGuard'; | |||
There was a problem hiding this comment.
Could just move this into the main layout however I've been breaking things out lmk what you think
| AssessmentQuestion, | ||
| } from '@/lib/types/candidate-assessment.types'; | ||
|
|
||
| export type AssessmentPhase = 'intro' | 'assessment' | 'outro'; |
There was a problem hiding this comment.
might be good to move these types into the candidate assessment types (thoughts?)
There was a problem hiding this comment.
I agree with this, types should def be managed in the same place
| } | ||
| } | ||
|
|
||
| async function setAssessmentStatus(status: AssessmentStatus) { |
There was a problem hiding this comment.
wait I'm realizing I prob shouldn't have removed this since we want to keep the admins up to date if the user took the oa (TODO for me btw)
There was a problem hiding this comment.
hmmm actually second thought this is fine - we don't have an in progress enum anyways so if font wants to know if someone is taking but I don't think that info is any useful rlly
LOTaher
left a comment
There was a problem hiding this comment.
Few requested changes. Want to chat during standup about a few things
| main: React.ReactNode; | ||
| }; | ||
|
|
||
| export default function AssessmentFlowLayout({ navbar, sidebar, main }: AssessmentFlowLayoutProps) { |
There was a problem hiding this comment.
Is this necessary? Seems like we can just inline this component no?
| @@ -0,0 +1,101 @@ | |||
| import { Loader2, CircleCheck, CircleX, AlarmClock } from 'lucide-react'; | |||
There was a problem hiding this comment.
I thought the purpose of us component-izing our task template page was to reuse our test case section? Would we be able to do that instead and just add extra parameters? If that's too big of an ask then I'd rather we combine these components and have a parameter to determine whether it should behave like its in an OA or editable
There was a problem hiding this comment.
Yeah we can talk abt this I realized that this might be hard to abstract away to use with the oa - but we can abstract for the task and assessment template stuff since its admin side data objects and we present similar info)
| @@ -0,0 +1,151 @@ | |||
| 'use client'; | |||
There was a problem hiding this comment.
Same feedback as the test case card
| @@ -0,0 +1,95 @@ | |||
| import { | |||
There was a problem hiding this comment.
Main seems somewhat unclear on first glance imo. Can we call this "AssessmentFlowScreen" or something?
| onStart: () => void; | ||
| }; | ||
|
|
||
| // move this to utils? (thoughts) |
There was a problem hiding this comment.
Can be a good call! We'd prob want to move formatDuration too
| @@ -0,0 +1,37 @@ | |||
| import { CircleCheck, Lock } from 'lucide-react'; | |||
There was a problem hiding this comment.
Can you call this something like "AssessmentFlowSidebarQuestion" or "AssessmentSidebarQuestion"
| @@ -0,0 +1,91 @@ | |||
| 'use client'; | |||
There was a problem hiding this comment.
Same feedback as the new test case component
| AssessmentQuestion, | ||
| } from '@/lib/types/candidate-assessment.types'; | ||
|
|
||
| export type AssessmentPhase = 'intro' | 'assessment' | 'outro'; |
There was a problem hiding this comment.
I agree with this, types should def be managed in the same place
| try { | ||
| const { assessmentId } = await params; | ||
| await AssessmentService.submitAssessmentForCandidate(assessmentId); | ||
| return Response.json({ data: null }, { status: 200 }); |
There was a problem hiding this comment.
This should prob be a 201 🤓
|
|
||
| export async function submitCandidateAssessment(assessmentId: string): Promise<void> { | ||
| const res = await fetch(`/api/oa/${assessmentId}/submit`, { method: 'PUT' }); | ||
| const json = await res.json(); |
There was a problem hiding this comment.
This should return something because the route returns data.
3b91025 to
b87633d
Compare
| ); | ||
| } | ||
|
|
||
| // the unique link shouldn't be the full link (bc the env of the link matters) so we just use id |
There was a problem hiding this comment.
Note: we've talked about this, this will just be a set of random characters for the url
Changes
Created the assessment flow (mainly the middle section but added a simple intro and outro part)
Notes
refreshing doesn't auto submit (I think I need to make an edit to the schema to have a start time and start the assessment
It does auto submit if we run out of time tho
No longer using routed intro outro and task bc having it in a single flow made state (such as the timer) easier to keep track of)
didn't reuse the testcase and code editor panels from the previous task and assessment template editor bc the data that we take in is different and what we show is also different (different enough to warrant new components) we'd essentially be creating new components within them imo)
Screen
https://sandboxneu.slack.com/archives/C09E7L2RK16/p1775078268444749?thread_ts=1775078043.152429&cid=C09E7L2RK16
Checklist
Please go through all items before requesting reviewers:
Closes
Closes #261